Emit references instead of definitions for object literal properties #256
Conversation
olafurpg
force-pushed
the
olafurpg/object-literals
branch
from
e0f5f2e to
cac25e3
Compare
|
Resolved everything here; this is some of the hackiest stuff I've put together in a while so heads up - it seems to work though :) |
olafurpg
force-pushed
the
olafurpg/object-literals
branch
2 times, most recently
from
6b66709 to
12c600f
Compare
|
Finally got a moment to look again into this PR and fixed one big remaining blocker to merge this PR. This PR is big and I want to make the changes low risk to merge as possible. I will need a few more iterations before it's safe to merge but I'm eager to get it through the finish line since it fixes a ton of very common navigation quirks. |
olafurpg
force-pushed
the
olafurpg/object-literals
branch
from
a11214c to
bf1b590
Compare
|
@valerybugakov @SuperAuguste this PR is ready for review now. I cleaned up the commit history so that you can only review the differences in the indexed output by looking at the diff of the last commit. The logic in |
|
What prevents us from merging this? |
|
@rodion-m main reason this hasn't been merged yet is because I'm personally working on other projects and haven't found the time to do the rebase, find reviewers, cut a release and communicate the changes (along with addressing followup feedback). I think the changes in this PR are a significant improvement to the TypeScript/JavaScript navigation experience. I'd love to see this land eventually but don't have an ETA for it at the moment. |
Simpler alternative to #256 This follows the implementation I found in the TypeScript goToDefinition service https://github.com/microsoft/TypeScript/blob/88809467e8761e71483e2f4948ef411d8e447188/src/services/goToDefinition.ts#L307-L316 Leans on the TypeScript compilers getContextualType function to resolve object literals to the interfaces, or type definitions they are being checked against. This means we generate useful references in a lot more places, where we'd generate "dead" definitions before. Test plan Ports all tests from #256 and adds some basic new ones. Check the updated snapshot output to gauge the improvement or if there are any regressions.
|
Superseeded by #373 Thanks a lot for all the great test cases in this PR! |
Fixes #252.
Previously, scip-typescript emitted definitions for properties of object literal expressions that got inferred to a concrete type. This was problematic since doing "Find references" didn't behave like it does in VS Code, where it shows the object literal property together with the type's member.
For example, consider
After this PR, the
propertyinproperty: 42becomes a regular reference toConfig.propertyinstead of being its own definition. This means that "Find references" now works like it does in VS Code.Test plan
See updated snapshots.